fix: charge collateral when a CoinJoin session aborts - #7568

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/coinjoin/coinjoin.h`:
- Around line 345-350: Update the comments above nSessionDenom to accurately
explain that tinyformat uses the atomic’s implicit int conversion, LogPrint()
accepts it by const reference without copying, and WalletCJLogPrint() forwards
to CWallet::WalletLogPrintf with by-value parameters, requiring an explicit
.load().

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/coinjoin/server.cpp`:
- [BLOCKING] src/coinjoin/server.cpp:625-628: Exclude in-flight submissions from guaranteed timeout penalties
  `SelectCollateralToCharge()` only recognizes entries already committed to `vecEntries`, but `AddEntry()` releases `cs_coinjoin` while running `IsCollateralValid()` and `IsValidInOuts()` at lines 759-794. A valid `DSVIN` whose processing began before the deadline can therefore still be validating when the scheduler observes the timeout. With three reservations, two committed entries, and the third entry in flight, the third participant is classified as the only missing submitter, selected with certainty, and charged after `SetNull()` makes its final session revalidation fail. Signing has the same gap while `DSSIGNFINALTX` is being decoded or between its per-input `AddScriptSig()` calls. Track in-flight messages for the current session or serialize the timeout cutoff with their complete processing before applying a guaranteed collateral penalty.
- [BLOCKING] src/coinjoin/server.cpp:610-628: Preserve recoverable finalization when the preceding pool check is skipped
  `CheckTimeout()` assumes the immediately preceding `CheckPool()` handled every recoverable accepting-entry timeout, but `CheckPool()` uses a non-blocking `TRY_LOCK`. A message-handling thread can hold `cs_check_pool`, sample `HasTimedOut()` as false immediately before the deadline, and then release the mutex after the scheduler's `CheckPool()` has skipped it but before the scheduler calls `CheckTimeout()`. `CheckTimeout()` then acquires the mutex after the deadline and unconditionally resets the session. If the session has at least `GetMinPoolParticipants()` committed entries but fewer entries than reservations, it should enter `ChargeAndFinalize` and retain the probabilistic policy; this interleaving instead aborts it and applies `GUARANTEED_ON_ABORT`. Re-evaluate the full accepting-entry action after acquiring `cs_check_pool` rather than relying on a preceding check that may have been skipped or sampled an earlier time.
- [BLOCKING] src/coinjoin/server.cpp:634: Fold the stray-brace correction into its introducing commit
  Commit `67c9647ed03` introduces an extra closing brace immediately after `CCoinJoinServer::CheckTimeout()`. The unmatched brace remains in `264ba3fdfa7`, `76e366e1ceb`, and `9e199087637`, and is only removed by the final commit `2bc5dbd248b`. Those four intermediate commits are syntactically unbuildable and unusable as `git bisect` points. Amend `67c9647ed03` to omit the extra brace, remove the corrective deletion from the final commit, and rebase the intervening commits so every permanent-history state builds independently.
- [SUGGESTION] src/coinjoin/server.cpp:503-509: Make offender deduplication explicit in the commit history
  Commit `9e199087637` is titled `refactor: separate CoinJoin offender selection policy`, but it also changes observable behavior: signing offenders are added once per entry instead of once per unsigned input. This removes the extra random-selection weight previously assigned to participants with multiple unsigned inputs. Move the deduplication hunk into the behavioral fix commit or reword `9e199087637` and add a commit body explaining the weighting change so `git log` and `git blame` expose the functional change.

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/coinjoin/server.cpp`:
- [BLOCKING] src/coinjoin/server.cpp:681-684: Do not guarantee a fee after the coordinator aborts signing
  The guaranteed timeout policy applies even after the server has instructed connected clients to stop signing. `RelayFinalTransaction()` calls `RelayStatus(STATUS_REJECTED)` when any entry is disconnected, and `ProcessDSSIGNFINALTX()` does the same after any `AddScriptSig()` failure. Connected clients process that rejection by entering `POOL_STATE_ERROR` and releasing their session resources, but the server remains in `POOL_STATE_SIGNING` unless every entry is disconnected. A malicious participant can exploit this by submitting its valid signature and then resubmitting it: the duplicate fails, every honest peer is told to abort, and the malicious participant is excluded from the unsigned-offender set. At timeout, this block then guarantees that one honest participant is charged. A failed `DSFINALTX` relay can similarly charge a connected entry that never received the transaction. Coordinator-originated signing aborts must reset without the guaranteed fee, or eligibility must be limited to participants that received the final transaction and were not subsequently instructed to abort.
- [BLOCKING] src/coinjoin/server.cpp:669-684: Commit complete signatures when rechecking a timeout
  `CheckTimeout()` re-evaluates recoverable accepting-entry sessions but does not re-evaluate `IsSignaturesComplete()` for signing sessions. A scheduler `CheckPool()` can acquire `cs_check_pool`, observe the signatures as incomplete, and release `cs_coinjoin`. The final on-time `DSSIGNFINALTX` can then add its signature, skip its own `CheckPool()` because the scheduler still owns `cs_check_pool`, and clear its in-flight guard. When the scheduler subsequently enters `CheckTimeout()` after the deadline, there are no unsigned offenders, but this block still calls `SetNull()` and discards the fully signed transaction. Re-evaluate the signing action under `cs_coinjoin`, then call `CommitFinalTransaction()` after releasing that lock, just as the accepting-entry action is re-evaluated and finalized.

In `<commit:aaa6d04>`:
- [SUGGESTION] <commit:aaa6d04>:1: Fold the lint-only follow-up into its introducing commits
  Commit `aaa6d0464a4` only adds `/* Continued */` markers to four `LogPrint` calls introduced earlier in this stack: one in `96cf3768cab` and three in `7443d022e09`. The linter rejects those unmarked calls, so retaining the separate correction leaves the introducing revisions as lint-failing bisect points and adds review-fix noise to permanent history. Fold each marker into the commit that introduced its call and drop `aaa6d0464a4`.

In `<commit:bb056c6>`:
- [SUGGESTION] <commit:bb056c6>:1: Preserve the rationale for the non-obvious CoinJoin fixes
  Commits `bb056c6d397`, `7da61132813`, and `3d5cf6b6e0f` have empty bodies despite changing subtle concurrency and fee-policy invariants. In particular, they serialize timeout reset with finalization, bind entry validation to a session denomination snapshot, and combine guaranteed abort charging with in-flight-message deferral and recoverable-timeout re-evaluation. Add concise bodies explaining the race or policy invariant each commit preserves so ordinary `git log`, blame, and bisect retain the reasoning currently available only from source comments and PR discussion.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/test/coinjoin_inouts_tests.cpp`:
- Around line 750-781: In server_timeout_rechecks_recoverable_session,
explicitly narrow the size_t loop variable when passing it to
MakeCollateral(uint32_t), and explicitly cast the computed address value passed
to MakePeer(NodeId, uint32_t). Preserve the existing loop behavior while
eliminating implicit conversion warnings.
- Around line 188-244: Add EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin) annotations to
ResetForTest, SetTimedOutForTest, AddCollateralForTest, AddEntryForTest,
SelectForTest, EnterSigningState, SetFinalTransactionForTest,
SeedTimedOutSession, and MarkMessageInFlightForTest, matching the existing
annotations on SeedParticipant and RelayAbortForTest. Keep each helper’s
internal cs_coinjoin locking unchanged.
- Around line 321-424: Split server_abort_fee_selects_unique_offenders into
separate BOOST_AUTO_TEST_CASE blocks covering each policy scenario, including
timeout handling, submission coverage, non-signer selection and weighting, the
probabilistic gate, and ChargeRandomFees. Move shared server setup into a small
helper to avoid repetition, and retain each scenario’s existing assertions and
setup behavior.
- Around line 609-624: Replace the floating-point expected amount in the delta
assertion after ApplyDelta with the exact integer expression COIN / 10, while
preserving the existing CAmount comparison and test behavior.